Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #883

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Solution #883

wants to merge 3 commits into from

Conversation

yulia-pl
Copy link

No description provided.

@yulia-pl
Copy link
Author

image
image
image
image

taxi/models.py Outdated
Comment on lines 37 to 42
class VisitCounter(models.Model):
num_visits = models.PositiveIntegerField(default=0)

def increment(self):
self.num_visits += 1
self.save()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need it here? Your counter should be on session layer, so you shouldn't create this model

taxi/views.py Outdated
Comment on lines 40 to 56
def login_view(request):
if request.method == "POST":
form = AuthenticationForm(request, data=request.POST)
if form.is_valid():
user = form.get_user()
login(request, user)
return redirect("taxi:index")
else:
form = AuthenticationForm()

return render(request, "taxi/../templates/registration/login.html",
{"form": form})


def logout_view(request):
logout(request)
return redirect("taxi:index")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create login/logout screens. Provide authentication to your project using built-in Django authentication.

Use bult-in authentication from django.contrib.auth.urls, fix this

taxi/views.py Outdated
Comment on lines 10 to 12
if not request.user.is_authenticated:
return redirect("taxi:login")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use @login_required decorator

taxi/views.py Outdated
Comment on lines 17 to 22
if "num_visits" not in request.session:
request.session["num_visits"] = 1
else:
request.session["num_visits"] += 1

num_visits = request.session["num_visits"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor this using method request.session.get() method

taxi/views.py Outdated
Comment on lines 34 to 38
@login_required
def driver_detail(request, pk):
driver = Driver.objects.get(pk=pk)
return render(request, "taxi/driver_detail.html", {"driver": driver})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already have DriverDetailView with generic why did you create this?

taxi/views.py Outdated
Comment on lines 59 to 108
class ManufacturerListView(generic.ListView):
model = Manufacturer
context_object_name = "manufacturer_list"
template_name = "taxi/manufacturer_list.html"
paginate_by = 5

def get(self, request, *args, **kwargs):
if not request.user.is_authenticated:
return redirect("taxi:login")
return super().get(request, *args, **kwargs)


class CarListView(generic.ListView):
model = Car
paginate_by = 5
queryset = Car.objects.select_related("manufacturer")
queryset = Car.objects.select_related("manufacturer").order_by("id")

def get(self, request, *args, **kwargs):
if not request.user.is_authenticated:
return redirect("taxi:login")
return super().get(request, *args, **kwargs)


class CarDetailView(generic.DetailView):
model = Car

def get(self, request, *args, **kwargs):
if not request.user.is_authenticated:
return redirect("taxi:login")
return super().get(request, *args, **kwargs)


class DriverListView(generic.ListView):
model = Driver
paginate_by = 5

def get(self, request, *args, **kwargs):
if not request.user.is_authenticated:
return redirect("taxi:login")
return super().get(request, *args, **kwargs)


class DriverDetailView(generic.DetailView):
model = Driver
queryset = Driver.objects.prefetch_related("cars__manufacturer")

def get(self, request, *args, **kwargs):
if not request.user.is_authenticated:
return redirect("taxi:login")
return super().get(request, *args, **kwargs)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read about LoginRequiredMixin and generics. You don't need to override get method here

@yulia-pl yulia-pl requested a review from AnyoneClown November 26, 2024 08:46
Copy link

@AnyoneClown AnyoneClown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left comments + in personal channel

Comment on lines +22 to 24
path("login/", auth_views.LoginView.as_view(), name="login"),
path("logout/", auth_views.LogoutView.as_view(), name="logout"),
]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to define this urls here. You should use already provided views from django auth.

You imported it in your settings app(main app).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants